Move sandbox run details to compact end-of-message card#1610
Move sandbox run details to compact end-of-message card#1610ivaavimusic wants to merge 4 commits intorecoupable:testfrom
Conversation
|
@ivaavimusic is attempting to deploy a commit to the Recoupable Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR introduces a new compact run details display system with collapsible UI components, supporting hooks, and utility functions. Changes include new components ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bad20d018e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| isStreaming={ | ||
| status === "streaming" && | ||
| partIndex === message.parts.length - 1 | ||
| status === "streaming" && partIndex === orderedParts.length - 1 | ||
| } |
There was a problem hiding this comment.
When deferred sandbox tool results are moved to the end, partIndex === orderedParts.length - 1 no longer identifies the actively streaming reasoning chunk in messages that continue with reasoning/text after a sandbox result. In that case EnhancedReasoning receives isStreaming=false while the assistant is still streaming, so shimmer/auto-collapse behavior can stop early and show reasoning as finalized before generation actually completes.
Useful? React with 👍 / 👎.
| const TERMINAL_STATUSES = new Set([ | ||
| "COMPLETED", | ||
| "FAILED", | ||
| "CRASHED", | ||
| "CANCELED", | ||
| "SYSTEM_FAILURE", | ||
| "INTERRUPTED", | ||
| ]); | ||
|
|
||
| const STATUS_BADGE_CLASSES: Record<string, string> = { | ||
| COMPLETED: | ||
| "border-green-200 bg-green-50 text-green-700 dark:border-green-900/60 dark:bg-green-950/50 dark:text-green-300", | ||
| FAILED: | ||
| "border-red-200 bg-red-50 text-red-700 dark:border-red-900/60 dark:bg-red-950/50 dark:text-red-300", | ||
| CRASHED: | ||
| "border-red-200 bg-red-50 text-red-700 dark:border-red-900/60 dark:bg-red-950/50 dark:text-red-300", | ||
| SYSTEM_FAILURE: | ||
| "border-red-200 bg-red-50 text-red-700 dark:border-red-900/60 dark:bg-red-950/50 dark:text-red-300", | ||
| INTERRUPTED: | ||
| "border-red-200 bg-red-50 text-red-700 dark:border-red-900/60 dark:bg-red-950/50 dark:text-red-300", | ||
| CANCELED: | ||
| "border-gray-200 bg-gray-50 text-gray-700 dark:border-gray-800 dark:bg-gray-950/50 dark:text-gray-300", | ||
| EXECUTING: | ||
| "border-yellow-200 bg-yellow-50 text-yellow-700 dark:border-yellow-900/60 dark:bg-yellow-950/50 dark:text-yellow-300", | ||
| REATTEMPTING: | ||
| "border-yellow-200 bg-yellow-50 text-yellow-700 dark:border-yellow-900/60 dark:bg-yellow-950/50 dark:text-yellow-300", | ||
| QUEUED: | ||
| "border-gray-200 bg-gray-50 text-gray-700 dark:border-gray-800 dark:bg-gray-950/50 dark:text-gray-300", | ||
| DELAYED: | ||
| "border-gray-200 bg-gray-50 text-gray-700 dark:border-gray-800 dark:bg-gray-950/50 dark:text-gray-300", | ||
| FROZEN: | ||
| "border-gray-200 bg-gray-50 text-gray-700 dark:border-gray-800 dark:bg-gray-950/50 dark:text-gray-300", | ||
| PENDING_VERSION: | ||
| "border-gray-200 bg-gray-50 text-gray-700 dark:border-gray-800 dark:bg-gray-950/50 dark:text-gray-300", | ||
| }; |
There was a problem hiding this comment.
Open Closed Principle
- actual: adding new const definitions to an existing RunDetails component.
- required: move new const definitions to new files.
There was a problem hiding this comment.
I added those constants directly while building the compact run view, but I agree they should be extracted instead of expanding RunDetails further. I’ll move them into dedicated files.
| import RunDetails from "@/components/TasksPage/Run/RunDetails"; | ||
| import RunPageSkeleton from "@/components/TasksPage/Run/RunPageSkeleton"; | ||
|
|
||
| function CompactRunSkeleton() { |
There was a problem hiding this comment.
Open Closed Principle
- actual: CompactRunSkeleton defined within RunSandboxCommandResultWithPolling
- required: new component file for CompactRunSkeleton
There was a problem hiding this comment.
Agreed. I’ll extract it into its own file.
| "prompt_sandbox", | ||
| ]); | ||
|
|
||
| function isDeferredSandboxResultPart( |
There was a problem hiding this comment.
Single Responsibility Principle
- actual: function isDeferredSandboxResultPart is defined within a component file.
- required: new standalone function file for isDeferredSandboxResultPart.
There was a problem hiding this comment.
Understood. That helper was added inline for the sandbox-result reordering, but I agree it should be extracted so MessageParts stays focused on rendering. I’ll move it into a standalone function file.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
components/TasksPage/Run/CompactRunDetails.tsx (2)
5-5: Verify icon-library consistency for this component tier.If this component is considered part of the UI-component layer, replace
lucide-reacthere with@radix-ui/react-iconsto match the repo convention.As per coding guidelines, "Use
@radix-ui/react-iconsfor UI component icons (not Lucide React)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/TasksPage/Run/CompactRunDetails.tsx` at line 5, This file imports ChevronDown from lucide-react which violates the repo guideline to use `@radix-ui/react-icons` for UI-component layer icons; replace the import of ChevronDown with the equivalent icon from `@radix-ui/react-icons` (or a suitable Radix icon name) in CompactRunDetails (and update any JSX references to that symbol if renamed), run a quick search in the component for ChevronDown usage to ensure the symbol name matches the new import, and update imports-only (no other logic changes) so the component consumes the Radix icon library consistently.
12-24: ExportCompactRunDetailsPropsfor consistency.The type name is good; exporting it would align with the project’s TS component API convention.
As per coding guidelines, "Export component prop types with explicit
ComponentNamePropsnaming convention".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/TasksPage/Run/CompactRunDetails.tsx` around lines 12 - 24, The CompactRunDetailsProps interface is currently unexported; update it to a named export so it follows the project's ComponentNameProps convention and can be reused externally—change the declaration of CompactRunDetailsProps to an exported interface (export interface CompactRunDetailsProps { ... }) in the CompactRunDetails.tsx file and keep the existing shape and name exactly the same.components/TasksPage/Run/RunDetails.tsx (2)
18-22: Export the props type for API clarity.
RunDetailsPropsalready uses the right naming; exporting it would align with the shared TS component contract pattern.As per coding guidelines, "Export component prop types with explicit
ComponentNamePropsnaming convention".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/TasksPage/Run/RunDetails.tsx` around lines 18 - 22, Export the RunDetailsProps interface so the prop type is available to other modules and matches the ComponentNameProps convention; update the declaration for RunDetailsProps to be exported (export interface RunDetailsProps { ... }) and ensure any usages/imports of the RunDetails component or its props (e.g., RunDetails) reference the exported RunDetailsProps type where needed.
36-50: Scope disclosure state to the compact variant path.
isOpenand theuseEffectrun even forvariant="full"where they are unused. Consider moving this state intoCompactRunDetails(or initializing only whenvariant === "chat-compact") to keep Line 36–50 variant-specific and reduce unnecessary state work.As per coding guidelines, "Create single-responsibility components with obvious data flow" and "Write minimal code - use only the absolute minimum code needed".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/TasksPage/Run/RunDetails.tsx` around lines 36 - 50, The open/close state and effect are being created in RunDetails even when variant !== "chat-compact"; move the isOpen useState and its useEffect into the compact-specific component (e.g., CompactRunDetails) or conditionally initialize them only when variant === "chat-compact" to avoid unnecessary state; specifically relocate/remove isOpen, setIsOpen, and the useEffect from RunDetails and implement the same logic inside CompactRunDetails (or wrap useState/useEffect in a guard checking variant) so the terminal-checking logic (isTerminal = TERMINAL_STATUSES.has(data.status)) and summaryText remain in RunDetails while compact-only UI state lives in the compact variant.components/VercelChat/tools/sandbox/CompactRunSkeleton.tsx (1)
3-12: Hide decorative skeleton blocks from screen readers.This skeleton is visual-only; consider marking the wrapper
aria-hidden="true"so assistive tech doesn’t read placeholder structure (Line 3).♻️ Suggested tweak
- <div className="w-full rounded-2xl border bg-background/80 px-4 py-3 shadow-sm"> + <div + aria-hidden="true" + className="w-full rounded-2xl border bg-background/80 px-4 py-3 shadow-sm" + >As per coding guidelines, "Provide proper ARIA roles/states and test with screen readers" and "Start with semantic HTML first, then augment with ARIA if needed".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/VercelChat/tools/sandbox/CompactRunSkeleton.tsx` around lines 3 - 12, The outer wrapper div in CompactRunSkeleton.tsx is purely decorative and should be hidden from assistive tech; update the top-level div (the one with className "w-full rounded-2xl border bg-background/80 px-4 py-3 shadow-sm") to include aria-hidden="true" so screen readers ignore the placeholder skeleton, and ensure none of the inner elements are focusable or contain interactive attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/TasksPage/Run/CompactRunDetails.tsx`:
- Around line 91-95: The external link rendering in the Link component (the JSX
element with href={`/tasks/${runId}`} and target="_blank`} in CompactRunDetails)
is missing the rel attribute; update that Link to include rel="noopener
noreferrer" to prevent tabnabbing and ensure safe external/new-tab behavior.
In `@components/VercelChat/MessageParts.tsx`:
- Around line 30-37: The render keys are unstable because getOrderedMessageParts
reorders originalParts but keys are derived from the rendered partIndex; change
getOrderedMessageParts to attach the original index (e.g., add originalIndex or
sourceIndex property to each returned part) and update all consumers (the render
loop and lastTextPartIndex computation) to use that preserved originalIndex as
the React key and for identity checks instead of the current partIndex; this
ensures components (e.g., expandable run card) keep stable identity when parts
are moved.
---
Nitpick comments:
In `@components/TasksPage/Run/CompactRunDetails.tsx`:
- Line 5: This file imports ChevronDown from lucide-react which violates the
repo guideline to use `@radix-ui/react-icons` for UI-component layer icons;
replace the import of ChevronDown with the equivalent icon from
`@radix-ui/react-icons` (or a suitable Radix icon name) in CompactRunDetails (and
update any JSX references to that symbol if renamed), run a quick search in the
component for ChevronDown usage to ensure the symbol name matches the new
import, and update imports-only (no other logic changes) so the component
consumes the Radix icon library consistently.
- Around line 12-24: The CompactRunDetailsProps interface is currently
unexported; update it to a named export so it follows the project's
ComponentNameProps convention and can be reused externally—change the
declaration of CompactRunDetailsProps to an exported interface (export interface
CompactRunDetailsProps { ... }) in the CompactRunDetails.tsx file and keep the
existing shape and name exactly the same.
In `@components/TasksPage/Run/RunDetails.tsx`:
- Around line 18-22: Export the RunDetailsProps interface so the prop type is
available to other modules and matches the ComponentNameProps convention; update
the declaration for RunDetailsProps to be exported (export interface
RunDetailsProps { ... }) and ensure any usages/imports of the RunDetails
component or its props (e.g., RunDetails) reference the exported RunDetailsProps
type where needed.
- Around line 36-50: The open/close state and effect are being created in
RunDetails even when variant !== "chat-compact"; move the isOpen useState and
its useEffect into the compact-specific component (e.g., CompactRunDetails) or
conditionally initialize them only when variant === "chat-compact" to avoid
unnecessary state; specifically relocate/remove isOpen, setIsOpen, and the
useEffect from RunDetails and implement the same logic inside CompactRunDetails
(or wrap useState/useEffect in a guard checking variant) so the
terminal-checking logic (isTerminal = TERMINAL_STATUSES.has(data.status)) and
summaryText remain in RunDetails while compact-only UI state lives in the
compact variant.
In `@components/VercelChat/tools/sandbox/CompactRunSkeleton.tsx`:
- Around line 3-12: The outer wrapper div in CompactRunSkeleton.tsx is purely
decorative and should be hidden from assistive tech; update the top-level div
(the one with className "w-full rounded-2xl border bg-background/80 px-4 py-3
shadow-sm") to include aria-hidden="true" so screen readers ignore the
placeholder skeleton, and ensure none of the inner elements are focusable or
contain interactive attributes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de087a20-f73a-4ddf-b35a-a0fda1c01d6a
📒 Files selected for processing (8)
components/TasksPage/Run/CompactRunDetails.tsxcomponents/TasksPage/Run/RunDetails.tsxcomponents/TasksPage/Run/runDetailsConstants.tscomponents/VercelChat/MessageParts.tsxcomponents/VercelChat/getOrderedMessageParts.tscomponents/VercelChat/isDeferredSandboxResultPart.tscomponents/VercelChat/tools/sandbox/CompactRunSkeleton.tsxcomponents/VercelChat/tools/sandbox/RunSandboxCommandResultWithPolling.tsx
| const displayDuration = useElapsedMs(data.startedAt, data.durationMs); | ||
| const isTerminal = TERMINAL_STATUSES.has(data.status); | ||
| const [isOpen, setIsOpen] = useState(!isTerminal); | ||
| const badgeClassName = | ||
| STATUS_BADGE_CLASSES[data.status] ?? | ||
| "border-border bg-muted/60 text-muted-foreground"; | ||
| const summaryText = | ||
| ERROR_STATUSES.has(data.status) && data.error?.message | ||
| ? data.error.message | ||
| : currentStep; | ||
|
|
||
| return ( | ||
| <div className="mx-auto flex flex-col gap-6 p-6"> | ||
| <div className="flex items-center gap-3"> | ||
| {config.icon} | ||
| <div> | ||
| {isOnRunPage ? ( | ||
| <h1 className="text-lg font-semibold">{displayName}</h1> | ||
| ) : ( | ||
| <Link | ||
| href={`/tasks/${runId}`} | ||
| target="_blank" | ||
| className="text-lg font-semibold hover:underline" | ||
| > | ||
| {displayName} | ||
| </Link> | ||
| )} | ||
| <p className={`text-sm ${config.color}`}>{config.label}</p> | ||
| </div> | ||
| </div> | ||
| useEffect(() => { | ||
| if (!isTerminal) { | ||
| setIsOpen(true); | ||
| } | ||
| }, [isTerminal]); |
There was a problem hiding this comment.
Open Closed Principle
- actual: new state added to existing component.
- required: new hook file for state needed in this component.
There was a problem hiding this comment.
Makes sense. I added that state there while wiring the compact chat behavior, but I agree it should be moved out into a dedicated hook so RunDetails stays lighter
There was a problem hiding this comment.
Functions should be stored in lib, not components.
- actual: components/VercelChat/getOrderedMessageParts.ts
- required: lib/vercel/getOrderedMessageParts.ts
There was a problem hiding this comment.
Understood. This is helper logic rather than UI, so I’ll move it out of components and into lib.
There was a problem hiding this comment.
Functions should be stored in lib, not components.
- actual: components/VercelChat/isDeferredSandboxResultPart.ts
- required: lib/vercel/isDeferredSandboxResultPart.ts
There was a problem hiding this comment.
Understood. Same here. This is function-level logic, so I’ll move it into lib as well.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| export default function RunDetails({ | ||
| runId, | ||
| data, | ||
| variant = "full", |
There was a problem hiding this comment.
Why are we adding a variant at all? What problem is this solving for musicians using Recoup?
There was a problem hiding this comment.
The goal here was to make sandbox run status easier to notice in chat without dropping the full run layout inline.
My intention was to keep it compact under one tile and place it at the end of the response so it is easier to notice without cluttering the main chat content. I used a variant to reuse the existing run details surface for chat, but I can split the chat version into a separate component if you’d prefer.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
components/TasksPage/Run/RunDetailsContent.tsx (1)
24-60: Use a single semantic root container for this details component.Line [24] currently returns a fragment with multiple roots. Wrap the content in one semantic container (e.g.,
<section>) and use heading elements for section titles to align with accessibility/structure conventions.♻️ Proposed refactor
export default function RunDetailsContent({ runId, data, }: RunDetailsContentProps) { const logs = data.metadata?.logs ?? []; const currentStep = data.metadata?.currentStep; return ( - <> + <section aria-label="Run details" className="flex flex-col gap-4"> <RunTimeline createdAt={data.createdAt} startedAt={data.startedAt} finishedAt={data.finishedAt} durationMs={data.durationMs} /> {currentStep && ( - <div className="rounded-md border bg-muted/30 px-4 py-2"> - <p className="text-xs font-medium uppercase tracking-wider text-muted-foreground"> + <section className="rounded-md border bg-muted/30 px-4 py-2"> + <h2 className="text-xs font-medium uppercase tracking-wider text-muted-foreground"> Current Step - </p> + </h2> <p className="text-sm font-medium">{currentStep}</p> - </div> + </section> )} {data.status === "COMPLETED" && data.output !== undefined && ( <RunOutput output={data.output} /> )} - <div className="flex-1 overflow-hidden"> - <p className="mb-2 text-xs font-medium uppercase tracking-wider text-muted-foreground"> + <section className="flex-1 overflow-hidden"> + <h2 className="mb-2 text-xs font-medium uppercase tracking-wider text-muted-foreground"> Activity Log - </p> + </h2> <RunLogsList logs={logs as string[]} /> - </div> + </section> {ERROR_STATUSES.has(data.status) && data.error && ( <RunErrorDetails error={data.error} /> )} <div className="text-xs text-muted-foreground"> <AccountIdDisplay accountId={runId} label="Run" /> </div> - </> + </section> ); }As per coding guidelines, "Use semantic HTML elements appropriate to the component's role" and "Wrap a single HTML element per component (no multiple root elements)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/TasksPage/Run/RunDetailsContent.tsx` around lines 24 - 60, The component currently returns a fragment with multiple sibling roots; wrap everything returned by the component in a single semantic root (e.g., a <section>) and replace the standalone paragraph labels with appropriate heading elements for accessibility (use <h2>/<h3> as appropriate) around the "Current Step" and "Activity Log" titles; ensure existing components RunTimeline, RunOutput, RunLogsList, RunErrorDetails and AccountIdDisplay remain inside that single root and that conditional renderings (currentStep, data.status === "COMPLETED", ERROR_STATUSES.has(data.status)) and the runId/data props are unchanged.components/VercelChat/MessageParts.tsx (1)
42-51: Reference equality for streaming detection may have subtle timing differences.The change from index-based (
partIndex === message.parts.length - 1) to reference equality (part === lastOriginalPart) is functionally equivalent for most cases, but there's a subtle difference:
lastOriginalPartis computed once fromoriginalPartsat render start- If
originalPartsreference changes mid-render (unlikely but possible with concurrent features),part === lastOriginalPartcould mismatchGiven that
useAutoCollapse(from the relevant snippets) has effects triggered onisStreamingtransitions, this could affect auto-collapse timing if the reference changes unexpectedly.This is likely fine in practice, but worth noting for awareness—particularly if you observe any flicker in the reasoning panel collapse behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/VercelChat/MessageParts.tsx` around lines 42 - 51, The streaming-detection switch to reference equality (part === lastOriginalPart) can be fragile if originalParts references change mid-render; switch to a stable index-based check instead: compute the last part index from originalParts (e.g., lastOriginalIndex = originalParts.length - 1) and use partIndex === lastOriginalIndex for the isStreaming condition in the EnhancedReasoning render, or otherwise derive a stable identifier per part; update the code paths involving EnhancedReasoning, lastOriginalPart, and useAutoCollapse to rely on that stable index/ID so auto-collapse behavior remains deterministic.lib/vercel/isDeferredSandboxResultPart.ts (1)
1-26: Clean predicate implementation with strong adherence to Single Responsibility Principle.This utility demonstrates solid design—it answers precisely one question: "Is this part a deferred sandbox result?" The use of a
Setfor tool name lookup is idiomatic and efficient for membership checks.Consider adding an explicit return type annotation for clarity:
Suggested improvement
export function isDeferredSandboxResultPart( part: UIMessagePart<UIDataTypes, UITools>, -) { +): boolean {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/vercel/isDeferredSandboxResultPart.ts` around lines 1 - 26, Add an explicit boolean return type to the isDeferredSandboxResultPart function to make its intent and API clearer; update the function signature for isDeferredSandboxResultPart(part: UIMessagePart<UIDataTypes, UITools>): boolean so it explicitly returns a boolean while keeping the existing body (which checks isToolOrDynamicToolUIPart, (part as ToolUIPart).state === "output-available", and DEFERRED_SANDBOX_TOOL_NAMES.has(getToolOrDynamicToolName(part))).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/VercelChat/MessageParts.tsx`:
- Around line 111-118: Tool component renders use an inconsistent key
(toolCallId) while other parts use a stable key based on originalIndex, breaking
React reconciliation; update the two tool renderers to accept and use the stable
key instead of emitting their own key: change getToolCallComponent and
getToolResultComponent signatures to accept a stableKey parameter and use that
for the React element key, and in MessageParts.tsx where you detect
isToolOrDynamicToolUIPart (ToolUIPart) pass the computed stable key (the one
derived from originalIndex) into those calls instead of relying on toolCallId;
ensure you still reference ToolUIPart state logic (state === "output-available")
and preserve existing returned markup but with the passed-in stableKey used for
keying.
---
Nitpick comments:
In `@components/TasksPage/Run/RunDetailsContent.tsx`:
- Around line 24-60: The component currently returns a fragment with multiple
sibling roots; wrap everything returned by the component in a single semantic
root (e.g., a <section>) and replace the standalone paragraph labels with
appropriate heading elements for accessibility (use <h2>/<h3> as appropriate)
around the "Current Step" and "Activity Log" titles; ensure existing components
RunTimeline, RunOutput, RunLogsList, RunErrorDetails and AccountIdDisplay remain
inside that single root and that conditional renderings (currentStep,
data.status === "COMPLETED", ERROR_STATUSES.has(data.status)) and the runId/data
props are unchanged.
In `@components/VercelChat/MessageParts.tsx`:
- Around line 42-51: The streaming-detection switch to reference equality (part
=== lastOriginalPart) can be fragile if originalParts references change
mid-render; switch to a stable index-based check instead: compute the last part
index from originalParts (e.g., lastOriginalIndex = originalParts.length - 1)
and use partIndex === lastOriginalIndex for the isStreaming condition in the
EnhancedReasoning render, or otherwise derive a stable identifier per part;
update the code paths involving EnhancedReasoning, lastOriginalPart, and
useAutoCollapse to rely on that stable index/ID so auto-collapse behavior
remains deterministic.
In `@lib/vercel/isDeferredSandboxResultPart.ts`:
- Around line 1-26: Add an explicit boolean return type to the
isDeferredSandboxResultPart function to make its intent and API clearer; update
the function signature for isDeferredSandboxResultPart(part:
UIMessagePart<UIDataTypes, UITools>): boolean so it explicitly returns a boolean
while keeping the existing body (which checks isToolOrDynamicToolUIPart, (part
as ToolUIPart).state === "output-available", and
DEFERRED_SANDBOX_TOOL_NAMES.has(getToolOrDynamicToolName(part))).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0ccdad50-6297-4519-8b3d-1ff960c81aa0
📒 Files selected for processing (10)
components/TasksPage/Run/CompactRunDetails.tsxcomponents/TasksPage/Run/RunDetails.tsxcomponents/TasksPage/Run/RunDetailsContent.tsxcomponents/VercelChat/MessageParts.tsxcomponents/VercelChat/tools/sandbox/ChatSandboxRunDetails.tsxcomponents/VercelChat/tools/sandbox/CompactRunSkeleton.tsxcomponents/VercelChat/tools/sandbox/RunSandboxCommandResultWithPolling.tsxhooks/useCompactRunDisclosure.tslib/vercel/getOrderedMessageParts.tslib/vercel/isDeferredSandboxResultPart.ts
✅ Files skipped from review due to trivial changes (1)
- components/VercelChat/tools/sandbox/CompactRunSkeleton.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- components/VercelChat/tools/sandbox/RunSandboxCommandResultWithPolling.tsx
- components/TasksPage/Run/CompactRunDetails.tsx
| if (isToolOrDynamicToolUIPart(part)) { | ||
| const { state } = part as ToolUIPart; | ||
| if (state !== "output-available") { | ||
| return getToolCallComponent(part as ToolUIPart); | ||
| } else { | ||
| return getToolResultComponent(part as ToolUIPart); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how getToolCallComponent and getToolResultComponent handle keys
rg -n -A 10 'export.*function getToolCallComponent|export.*function getToolResultComponent' components/VercelChat/ToolComponents.tsxRepository: recoupable/chat
Length of output: 1032
🏁 Script executed:
# Get full implementation of getToolCallComponent
rg -n -A 200 'export function getToolCallComponent' components/VercelChat/ToolComponents.tsx | head -250Repository: recoupable/chat
Length of output: 6081
🏁 Script executed:
# Get full implementation of getToolResultComponent
rg -n -A 200 'export function getToolResultComponent' components/VercelChat/ToolComponents.tsx | head -250Repository: recoupable/chat
Length of output: 6808
🏁 Script executed:
# Get the map code in MessageParts.tsx to see the full context
sed -n '27,65p' components/VercelChat/MessageParts.tsxRepository: recoupable/chat
Length of output: 1324
Inconsistent key strategy for tool components breaks React reconciliation pattern.
The getToolCallComponent and getToolResultComponent functions already include key={toolCallId} in their returns. However, the problem is that the map computes a stable key based on originalIndex (used for text, reasoning, and file parts) but tool components ignore this and use toolCallId instead. This inconsistency breaks the reconciliation guarantees across the message parts list.
All components in the same map() should use the same keying strategy. Update the tool component functions to accept the stable key as a parameter:
- export function getToolCallComponent(part: ToolUIPart) {
+ export function getToolCallComponent(part: ToolUIPart, key: string) {
const { toolCallId } = part as ToolUIPart;
// ... inside returns:
- <div key={toolCallId}>
+ <div key={key}>Then pass the computed key in MessageParts.tsx:
if (isToolOrDynamicToolUIPart(part)) {
const { state } = part as ToolUIPart;
if (state !== "output-available") {
- return getToolCallComponent(part as ToolUIPart);
+ return getToolCallComponent(part as ToolUIPart, key);
} else {
- return getToolResultComponent(part as ToolUIPart);
+ return getToolResultComponent(part as ToolUIPart, key);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/VercelChat/MessageParts.tsx` around lines 111 - 118, Tool
component renders use an inconsistent key (toolCallId) while other parts use a
stable key based on originalIndex, breaking React reconciliation; update the two
tool renderers to accept and use the stable key instead of emitting their own
key: change getToolCallComponent and getToolResultComponent signatures to accept
a stableKey parameter and use that for the React element key, and in
MessageParts.tsx where you detect isToolOrDynamicToolUIPart (ToolUIPart) pass
the computed stable key (the one derived from originalIndex) into those calls
instead of relying on toolCallId; ensure you still reference ToolUIPart state
logic (state === "output-available") and preserve existing returned markup but
with the passed-in stableKey used for keying.
2 participants
Summary
This PR improves how sandbox task results appear inside chat responses.
Changes
Why
Inline sandbox details were easy to miss and visually heavy. This makes the run status easier to scan while still keeping full logs and error details available on demand.
Risk
Low to medium. The change is scoped to chat rendering for sandbox task results and does not alter backend task execution.
Summary by CodeRabbit
New Features
Refactor